add isBackgroundWorker, databaseId and roleId to pg_wait_sampling_current view #95
Conversation
DmitryNFomin
force-pushed
the
collect_more_backend_info
branch
3 times, most recently
from
2c8514d to
ec861c5
Compare
There was a problem hiding this comment.
In your commit you have not updated output of pg_wait_sampling[_get]_profile but it was considered at first. You have also added "some" fields, and while those are good and will be probably added in some way, why not other fields? Why not "application name" for example?
Also we have no way to turn those fields off. While not as important for _current and _history functions, for _profile it would be bad, since you break aggregation by query_id and/or proc_pid by adding additional dimensions. And we have probably decided that some additional fields will be added to _profile (and to _current and _history as well), but we are still thinking of a way to add those with on/off switch and to keep pg_wait_sampling lightweight
pg_wait_sampling--1.0--1.1.sql
Outdated
There was a problem hiding this comment.
If you're changing structure of some table or changing input/output parameters you have to make a new SQL version of your extension to not break existing installations
So you have to add pg_wait_sampling--1.1--1.2.sql
There was a problem hiding this comment.
Also, why did you change tabs for spaces before in/out parameters?
There was a problem hiding this comment.
@Medvecrab
thanks a lot for you review.
I wanted to add fields that could help us identify backend. Yes, application name, backend type (and probably query text) are ideal, but for that we will need to work with backend status structure, which needs additional locking and will increase observer effect.
So I added fields from PGPROC that would help us to identify backends but would not lead to significant increase of observer effect.
I added this to _current and _history views only because in _profile we group by pid while theoretically (probability is low) we could have more than 1 process with same pid, so if we set this fields to pid in _profile it can be misleading in some rare cases
There was a problem hiding this comment.
all correction you suggested I've implemented
|
So for now we will think of how to carefully add new fields that were discussed in #94 (maybe all of them, maybe some of them, maybe will add some additional ones) and will add them ourselves. Not closing this pull request yet but it probably won't be merged |
DmitryNFomin
force-pushed
the
collect_more_backend_info
branch
from
ec22496 to
12946dc
Compare
I would be happy as a user to have all this fields in new version, as far as I can understand some changes needs more work while this change is quite simple and probably we can have merged this while more complicated changes are being worked on. anyway many thanks for your extension, my personal view that wait event analysis should take centra position l in postgres load profile assessment and troubleshooting, like ASH in Oracle |
For better visibility we need to know type of the backend of pid that we collected wait info.
It can be done by joining with pg_stat_activity view by pid, but this is not possible for short living backends, pid does not exist anymore when exporter or some other process comes to collect data
To address this problem I would like to add data that we can get from PGPROC that can help identify backend (at least if was regular backend and if yes which database and role)
It's just to explain idea, if you find it good idea I will add code for history and profile views